Repository navigation
feat(alerts): record insight alert investigations for platform evaluations - #123622
Conversation
…tions The platform never writes AlertCheck, so an investigation of a platform evaluation needs its own row. InsightAlertInvestigation holds it, and the budget, carried verdict and claim rules are ported to read it, keyed on the platform configuration and its firing's start. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
😎 Stack merged successfully - details. |
🤖 CI report
|
| File | Comment lines | Added lines |
|---|---|---|
products/alerts/backend/models/investigation.py |
8 | 59 |
products/alerts/backend/platform_investigation.py |
3 | 91 |
products/alerts/backend/tests/test_platform_investigation.py |
2 | 133 |
This check does not block merging. It updates on every push and clears when the share drops.
⚠️ Django migration SQL — 1 new migration to review
We've detected new migrations on this PR. Review the SQL output for each migration:
products/alerts/backend/migrations/0015_insight_alert_investigation.py
/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/anyio/from_thread.py:119: SyntaxWarning: 'return' in a 'finally' block
return result
2026-10-27T04:47:52.414510Z [error ] Path must be a valid database or directory containing databases. [posthog.exceptions_capture] pid=28963 tid=139632606714752
Traceback (most recent call last):
File "/home/runner/work/posthog/posthog/posthog/geoip.py", line 15, in <module>
geoip: Optional[GeoIP2] = GeoIP2(cache=8)
~~~~~~^^^^^^^^^
File "/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/django/contrib/gis/geoip2.py", line 116, in __init__
raise GeoIP2Exception(
"Path must be a valid database or directory containing databases."
)
django.contrib.gis.geoip2.GeoIP2Exception: Path must be a valid database or directory containing databases.
/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/sshtunnel.py:1040: SyntaxWarning: 'return' in a 'finally' block
return (ssh_host,
/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/langchain_core/_api/deprecation.py:27: UserWarning: Core Pydantic V1 functionality isn't compatible with Python 3.14 or greater.
from pydantic.v1.fields import FieldInfo as FieldInfoV1
System check identified some issues:
WARNINGS:
?: (axes.W001) You are using the django-axes cache handler for login attempt tracking. Your cache configuration is however invalid and will not work correctly with django-axes. This can leave security holes in your login systems as attempts are not tracked correctly. Reconfigure settings.AXES_CACHE and settings.CACHES per django-axes configuration documentation.
?: (staticfiles.W004) The directory '/home/runner/work/posthog/posthog/frontend/dist' in the STATICFILES_DIRS setting does not exist.
BEGIN;
--
-- Create model InsightAlertInvestigation
--
CREATE TABLE "alerts_insightalertinvestigation" ("id" uuid NOT NULL PRIMARY KEY, "configuration_id" uuid NOT NULL, "evaluation_key" varchar(255) NOT NULL, "episode_started_at" timestamp with time zone NULL, "kind" varchar(16) NOT NULL, "status" varchar(10) NOT NULL, "verdict" varchar(20) NULL, "summary" text NULL, "error" jsonb NULL, "evaluated" jsonb NOT NULL, "created_at" timestamp with time zone NOT NULL, "updated_at" timestamp with time zone NOT NULL, "alert_configuration_id" uuid NOT NULL, "notebook_id" uuid NULL, "team_id" integer NOT NULL, CONSTRAINT "insight_alert_investigation_one_per_evaluation" UNIQUE ("configuration_id", "evaluation_key"));
ALTER TABLE "alerts_insightalertinvestigation" ADD CONSTRAINT "alerts_insightalerti_alert_configuration__f6888e9c_fk_posthog_a" FOREIGN KEY ("alert_configuration_id") REFERENCES "posthog_alertconfiguration" ("id") DEFERRABLE INITIALLY DEFERRED;
CREATE INDEX "alerts_insightalertinvesti_alert_configuration_id_f6888e9c" ON "alerts_insightalertinvestigation" ("alert_configuration_id");
CREATE INDEX "alerts_insightalertinvestigation_team_id_ebd9673a" ON "alerts_insightalertinvestigation" ("team_id");
CREATE INDEX "insight_alert_inv_episode_idx" ON "alerts_insightalertinvestigation" ("configuration_id", "episode_started_at", "created_at");
CREATE INDEX "insight_alert_inv_notebook_idx" ON "alerts_insightalertinvestigation" ("notebook_id") WHERE "notebook_id" IS NOT NULL;
CREATE INDEX "insight_alert_inv_recent_idx" ON "alerts_insightalertinvestigation" ("configuration_id", "created_at");
COMMIT;Last updated: 2026-10-27 04:48 UTC (4673a22)
✅ Django migration risk — migration analysis complete
We've analyzed your migrations for potential risks.
Summary: 1 Safe | 0 Needs Review | 0 Blocked
✅ Safe
Brief or no lock, backwards compatible
alerts.0015_insight_alert_investigation
└─ #1 ✅ CreateModel
Creating new table is safe
model: InsightAlertInvestigation
│
└──> ℹ️ INFO:
ℹ️ Skipped operations on newly created tables (empty tables
don't cause lock contention).
Last updated: 2026-10-27 04:48 UTC (4673a22)
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
There was a problem hiding this comment.
PostHog Review · Standard · reviewed e75437a: 1 Should fix.
This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
…link Add InsightAlertInvestigation to the IDOR semgrep rules, declare the test class attributes for mypy, and index the notebook link (non-null rows only) so deleting a notebook does not scan the table. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Risk: No findings Since the last review the only authored change is a merge commit's edit to the semgrep IDOR model lists: InsightAlertInvestigation is now listed (alphabetically) in the two team-scoping enforcement rules, and SignalReportMonitoringResult entries that an earlier branch commit had dropped are restored. Both classifications match the actual model definitions, and no model was removed from any enforcement list, so the security rules are strengthened, not weakened. Sentinel reviewed |
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
/trunk merge |
…e hold model's Both this PR and the delivery-hold PR inserted a line at the same spot in the IDOR model lists, so whichever landed second conflicted. List the investigation model next to AlertConfiguration instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A new stamphog review started for this PR — the fresh verdict replaces this approval.
There was a problem hiding this comment.
Not approved yet — waiting on the conditions below.
The Migration risk check has not finished for this commit, so stamphog cannot tell a safe migration from a risky one yet. The review runs again on the next push, or you can re-request it once the check reports.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✗ | matches: migrations |
| size | ✓ | 358L, 7F substantive, 516L/9F incl. docs/generated/snapshots — within ceiling |
| tier | ✗ | classified as T2-never: T2-never (516L, 9F, cross-cutting, feat) |
| stamphog 2.6.1 | .stamphog/policy.yml @ unknown · reviewed head accca4e |
jzhu13
added
stamphog
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
/trunk merge |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…/insight-alert-investigation
|
/trunk cancel |
The PR was retargeted to a different base branch, so the approved diff is no longer what was reviewed. Stamphog re-reviews automatically.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
A new stamphog review started for this PR — the fresh verdict replaces this approval.
There was a problem hiding this comment.
Not approved yet — waiting on the conditions below.
The Migration risk check has not finished for this commit, so stamphog cannot tell a safe migration from a risky one yet. The review runs again on the next push, or you can re-request it once the check reports.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✗ | matches: migrations |
| size | ✓ | 356L, 6F substantive, 514L/8F incl. docs/generated/snapshots — within ceiling |
| tier | ✗ | classified as T2-never: T2-never (514L, 8F, two-areas, feat) |
| stamphog 2.6.1 | .stamphog/policy.yml @ unknown · reviewed head 7951940 |
jzhu13
added
stamphog
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The PR was retargeted to a different base branch, so the approved diff is no longer what was reviewed. Stamphog re-reviews automatically.
There was a problem hiding this comment.
Not approved yet — waiting on the conditions below.
The Migration risk check has not finished for this commit, so stamphog cannot tell a safe migration from a risky one yet. The review runs again on the next push, or you can re-request it once the check reports.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✗ | matches: migrations |
| size | ✓ | 356L, 6F substantive, 514L/8F incl. docs/generated/snapshots — within ceiling |
| tier | ✗ | classified as T2-never: T2-never (514L, 8F, two-areas, feat) |
| stamphog 2.6.1 | .stamphog/policy.yml @ unknown · reviewed head 4673a22 |
jzhu13
added
stamphog
There was a problem hiding this comment.
Approved.
The change enters schema, model, and write-path territory, with owning-team authorship providing an assurance path. No current-head review is provided; the older bot review names no concrete concern to verify, and the code shows no identified showstopper.
- Author wrote 0% of the modified lines and has 24 merged PRs in these paths (familiarity MODERATE).
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 356L, 6F substantive, 514L/8F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (514L, 8F, two-areas, feat) |
| stamphog 2.6.1 | .stamphog/policy.yml @ 4673a22 · reviewed head 4673a22 |
|
/trunk merge |
Problem
AlertCheck.AlertCheck, so a platform-evaluated alert has nowhere to record an investigation or read the last one.Changes
InsightAlertInvestigationtable: one row per platform evaluation that asked for an investigation, keyed on the platform configuration and evaluation key.episode_started_at, instead of a run of FIRINGAlertCheckrows.products/alerts/backend/platform_investigation.pyports the decision and the claim to the new table:decide_investigationis a read, so a retried evaluation can repeat it. It ignores the evaluation's own row.claim_investigationwrites PENDING or SKIPPED under a per-alert advisory lock. A repeated claim returns the first answer, so a retry starts the same agent.posthog/temporal/alerts/investigation.py. Both stacks call them, so the rules cannot drift during the parallel run.episode_started_atand count as one episode.AlertConfigurationis hidden (related_name="+"). With a visible one, code that saves an alert outside a team scope raisesTeamScopeError.How did you test this code?
Test rationale:
test_platform_investigation.pycovers what is new: the episode keying and own-row exclusion on the decision, the claim writing PENDING or SKIPPED through the shared refusal rule, and the repeated claim returning its first answer.posthog/temporal/tests/test_alerts_investigation.pykeep covering the shared rules in full, so the platform tests carry only the wiring cases.test_alerts_investigation.py,test_alerts_activities.py, and the fullproducts/alertssuite. Threeproducts/alertsAPI tests that read platform history fail locally because the local ClickHouse test schema predates thesource_detailcolumn on master. CI runs them against a fresh schema.Release status
Docs update
None. Nothing calls the new code yet.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Claude Code, Claude Opus 5.5
/django-migrations,/writing-tests,/code-review high --fix,/simplify,/reviewing-with-coderabbit,/writing-pr-descriptions./code-reviewcaught that the decision compared platform state names against the insight stack's casing; it now takesfiring: bool. Running the legacy tests caught the reverse-relationTeamScopeError./simplifymoved the rules into shared helpers and trimmed the duplicated test cases. Not taken: one table for both stacks, because the platform does not write insight-stack rows.--deep: no findings.